-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Faster wasm runtime #51458
Faster wasm runtime #51458
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
@radekdoulik is it possible to test the performance of this with your example in #50260 (comment); I'm not sure how to get a build that can be tested in your harness, I just seemed to make it angry in the console instead |
Tagging subscribers to 'arch-wasm': @lewing Issue Details
Trading off code size and performance
Inspired by Is WebAssembly magic performance pixie dust? where they were seeing a ore than x2 perf degradation from
And its currently using
|
src/mono/wasm/Makefile
Outdated
@@ -72,7 +72,7 @@ ifeq ($(ENABLE_METADATA_UPDATE),true) | |||
endif | |||
|
|||
EMCC_DEBUG_FLAGS =-g -Os -s ASSERTIONS=1 -DDEBUG=1 | |||
EMCC_RELEASE_FLAGS=-Oz --llvm-opts 2 | |||
EMCC_RELEASE_FLAGS=-O3 --llvm-opts 3 -flto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) not that it really matters, but the ordering here is different than what is in the wasm.proj file.
Does this need to pass Refers to: src/mono/wasm/Makefile:105 in 7ea1fa3. [](commit_id = 7ea1fa3, deletion_comment = False) |
That needs to be passed via |
I don't believe that Task gets called on non-Windows builds: runtime/src/mono/wasm/wasm.proj Lines 112 to 115 in 8f4a12d
vs. runtime/src/mono/wasm/wasm.proj Lines 74 to 80 in 8f4a12d
From my understanding the Makefile and the |
I am updating my measurement code, so that it becomes more automated and less hassle to use. I will try to run it on your branch and also create PR which will add new sample, similar to |
Added to the Makefile also |
Yes, indeed. I plan to unify that, hopefully soon. Opened #51553. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have measured the Json timings.
measurement | main/interp | PR branch/interp |
---|---|---|
Json, non-ASCII text serialize | 8.2714ms | 8.2941ms |
Json, non-ASCII text deserialize | 11.6825ms | 11.6508ms |
Json, small serialize | 0.2389ms | 0.2436ms |
Json, small deserialize | 0.3678ms | 0.3731ms |
Json, large serialize | 67.8831ms | 69.4267ms |
Json, large deserialize | 101.1538ms | 102.4038ms |
They are very close, probably just measurement errors. Not sure whether the change is too small or whether the Json [de]serialization is not affected by changes in this PR.
Any suggestion for an area where it might show larger difference?
This change might have more impact with AOT enabled ( I'm also wondering if this may help more the GC itself, what happens if you make lots of allocations during benchmarks? |
I think for impact on AOT we would need to also modify |
I have opened draft PR with the simple benchmark sample I used to measure the times #51559 |
I am sorry @benaadams that this fall off the radar. We switched to @radical, I think the flags can be overridden with |
-O3
rather than-Oz
for a faster runtime-flto
for extra optimizations--closure 1
which can greatly reduce the size of the JS and more than make up for the increase in the wasm (see: Faster, smaller wasm runtime #51446)Optimizing Code | Link Times
Trading off code size and performance
Inspired by Is WebAssembly magic performance pixie dust? where they were seeing a ore than x2 perf degradation from
-Os
:And its currently using
-Oz
which is even worse for performance than-Os